Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(lint): fix linter for multi-module repo #13

Merged
merged 2 commits into from
Apr 7, 2024

Conversation

madflojo
Copy link
Member

@madflojo madflojo commented Apr 7, 2024

Currently, golangci-lint is not checking sub-directories.

Summary by CodeRabbit

  • Chores
    • Improved CI linting process for better code quality checks across different modules.
  • Refactor
    • Added empty lines after import statements in multiple files for better code readability.
  • New Features
    • Added import statements for "html" and "emoji" packages in relevant files.

Copy link
Contributor

coderabbitai bot commented Apr 7, 2024

Walkthrough

The recent changes involve enhancements and adjustments across various files within the project. These updates primarily focus on refining import statements and formatting in different Go files to maintain consistency and improve readability without altering the core functionality significantly.

Changes

Files Change Summary
functions/src/data/fetch/main.go Added an empty line after import statements.
functions/src/data/init/main.go Added an empty line after the import statement for github.com/tarmac-project/tarmac/pkg/sdk.
functions/src/data/load/main.go Added import statement for "html" and minor comment formatting changes.
functions/src/handlers/lookup/main.go Added import statement for "html" and reordered existing import statements.
pkg/airport/airport.go Added import statement for the emoji package.
pkg/airport/parsers/csv/csv.go Adjusted import order, moving pkg/airport import below standard library imports.
pkg/airport/parsers/csv/csv_test.go Reorganized import statements for external packages with minor order changes.

🐇💻
In the land of code, where changes play,
A rabbit hopped along the import way.
Empty lines here, imports rearranged there,
With a touch of whimsy, the code felt fair.
"Importance in order," the rabbit did shout,
As the codebase danced, a harmonious clout.
🐾📦🌟

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 2e99061 and 4852b27.
Files selected for processing (1)
  • .github/workflows/lint.yml (2 hunks)
Additional comments not posted (2)
.github/workflows/lint.yml (2)

14-16: The addition of the strategy block with a matrix is a good approach to ensure golangci-lint checks all specified modules. Please verify that all relevant modules are included.


29-29: The update to dynamically set the working-directory is crucial for the effectiveness of the linting process across different modules. This change ensures that golangci-lint runs in the context of each specified module.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Actionable comments outside the diff hunks (2)
functions/src/handlers/lookup/main.go (1)

Line range hint 49-49: Use parameterized queries to prevent SQL injection.

- query := fmt.Sprintf(`SELECT * FROM airports WHERE local_code = "%s"`, html.EscapeString(lc))
+ query := `SELECT * FROM airports WHERE local_code = ?`
+ data, err := f.tarmac.SQL.Query(query, html.EscapeString(lc))

Direct string concatenation in SQL queries can lead to SQL injection vulnerabilities. Use parameterized queries instead to enhance security.

functions/src/data/load/main.go (1)

Line range hint 52-52: Use parameterized queries to enhance security.

- query := fmt.Sprintf(`INSERT INTO airports (...) VALUES (...) ON DUPLICATE KEY UPDATE ...;`, html.EscapeString(airport.LocalCode), ...)
+ query := `INSERT INTO airports (...) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON DUPLICATE KEY UPDATE ...;`
+ _, err := f.tarmac.SQL.Query(query, html.EscapeString(airport.LocalCode), ...)

Direct string concatenation in SQL queries can lead to SQL injection vulnerabilities. Use parameterized queries instead to enhance security.

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4852b27 and 2dac406.
Files selected for processing (7)
  • functions/src/data/fetch/main.go (1 hunks)
  • functions/src/data/init/main.go (1 hunks)
  • functions/src/data/load/main.go (1 hunks)
  • functions/src/handlers/lookup/main.go (1 hunks)
  • pkg/airport/airport.go (1 hunks)
  • pkg/airport/parsers/csv/csv.go (1 hunks)
  • pkg/airport/parsers/csv/csv_test.go (1 hunks)
Files skipped from review due to trivial changes (4)
  • functions/src/data/fetch/main.go
  • functions/src/data/init/main.go
  • pkg/airport/parsers/csv/csv.go
  • pkg/airport/parsers/csv/csv_test.go
Additional comments not posted (7)
functions/src/handlers/lookup/main.go (4)

11-11: Consider grouping standard library imports together.

It's a common Go convention to group imports into three sections: standard library, third-party packages, and internal packages. This enhances readability and maintainability. You might want to group the "html" import with other standard library imports.


Line range hint 39-41: Ensure proper error handling for missing local_code.

Returning both an error and a non-nil response might lead to ambiguous behavior. Consider standardizing the error handling approach to either return an error or a valid response, but not both.


Line range hint 73-73: Consider caching the response before returning.

You're adding the response to the cache after creating the JSON response. It might be more efficient to cache the data as soon as it's retrieved from the database, before processing it into the final JSON format. This could save processing time in future requests.


Line range hint 85-93: Optimize base64 decoding in decodeData.

You're decoding base64 strings inside a loop. If the decoding process is CPU-intensive and the data set is large, consider parallelizing this operation or using a more efficient method to handle the decoding.

pkg/airport/airport.go (1)

5-5: Import of emoji package added.

The addition of the emoji package import is consistent with the usage in the setTypeEmoji and setCountryFlag functions. This change enhances the functionality by allowing the representation of airport types and countries with emojis.

functions/src/data/load/main.go (2)

13-13: Consider grouping standard library imports together.

Similar to the previous file, it's a good practice in Go to group imports by their types: standard library, third-party packages, and internal packages. This enhances readability and maintainability. Grouping the "html" import with other standard library imports would align with this convention.


Line range hint 70-70: Consider handling database update failures more gracefully.

When a database update fails, logging the error is a good practice, but it might also be beneficial to implement a retry mechanism or log the specific data causing the failure for easier debugging and recovery.

@madflojo madflojo merged commit 1a82646 into tarmac-project:main Apr 7, 2024
7 checks passed
@madflojo madflojo deleted the fix-lint-action branch April 7, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant